Skip to content

Fix beta6 fact#384

Merged
truthbk merged 2 commits intoDataDog:masterfrom
vend:fix_beta6_fact
Feb 1, 2018
Merged

Fix beta6 fact#384
truthbk merged 2 commits intoDataDog:masterfrom
vend:fix_beta6_fact

Conversation

@scottgeary
Copy link
Copy Markdown

@scottgeary scottgeary commented Jan 15, 2018

In some situations facts are likely returned as a string. Any non-empty string can be interpreted as true when building a conditional. See https://puppet.com/docs/puppet/5.3/lang_facts_and_builtin_vars.html#handling-boolean-facts-in-older-puppet-versions

Ensure we're interpreting ::apt_agent6_beta_repo as a valid boolean, so we don't get half-baked package removals.

Example debug message confirming the various ways to interpret the fact:

Debug: Scope(Class[Datadog_agent::Ubuntu]): Fact ::apt_agent6_beta_repo=false type=string is_true=true str2bool=false

puppet --version
3.8.7

@scottgeary scottgeary mentioned this pull request Jan 15, 2018
Closed
@truthbk
Copy link
Copy Markdown
Member

truthbk commented Jan 31, 2018

Thank you for this fix @scottgeary

Tests are good but we've got some linting issues:

manifests/ubuntu.pp:40:only_variable_string:WARNING:string containing only a variable
manifests/ubuntu.pp:53:only_variable_string:WARNING:string containing only a variable
manifests/ubuntu.pp:40:variables_not_enclosed:WARNING:variable not enclosed in {}
manifests/ubuntu.pp:53:variables_not_enclosed:WARNING:variable not enclosed in {}

Can you address that? We could just add a lint:ignore:only_variable_string, because there's no real clean way of checking for types in older puppets, I know =~ doesn't work in 3.7.

Thanks a lot for this!

@truthbk truthbk added this to the 2.0.0 milestone Jan 31, 2018
@truthbk
Copy link
Copy Markdown
Member

truthbk commented Feb 1, 2018

I'll merge and fix the linting on my end, thanks @scottgeary

truthbk
truthbk approved these changes Feb 1, 2018
View reviewed changes
Copy link
Copy Markdown
Member

@truthbk truthbk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

merging! 🚀

@truthbk truthbk merged commit 142a858 into DataDog:master Feb 1, 2018
@truthbk truthbk mentioned this pull request Feb 1, 2018
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer

@truthbk truthbk truthbk approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

backport community

Projects

None yet

Milestone

2.0.0

Development

Successfully merging this pull request may close these issues.

2 participants

@scottgeary @truthbk
⚡